UI/DataTable: initialize with Order and Range #8106
Conversation
nhaagen
changed the title
nhaagen
force-pushed
the
UI/DataTable/initialize
branch
from
552c12f to
94658d1
Compare
There was a problem hiding this comment.
Hi @nhaagen,
Thx a lot for your contribution to the UI framework.
About the concrete changes, please answer the following questions:
-
Data::withNumberOfRows()usages: could you check if there are any usages of this method inside ILIAS? And, it it is not too much work, maybe directly replace these calls, so we can request reviews in this PR.
Otherwise this LGTM and should solve both mantis issues. Since we are removing an interface method, we need to go to JF before we can merge this.
Kind regards,
@thibsy
|
There were, in fact, but two occurences of "withNumberOfRows"; I replaced those. |
| )->withNumberOfRows( | ||
| $this->request->getItemsPerPage() | ||
| )->withRange( | ||
| new Range(0, $this->request->getItemsPerPage()) |
There was a problem hiding this comment.
@nhaagen Wouldn't that mean that only the first X entries are displayed? Or would the pagination itself then update that? I'm just a little confused about the static “0”.
There was a problem hiding this comment.
@nhaagen Ok, I think I understand it now, it's really just about the default values :-) That's fine with me, thank you very much!
|
@nhaagen just noticed there's something wrong with the code-style of the changed files. Could you amend this, before we merge? Thx! |
|
Jour Fixe, 30 SEP 2024: We accept the suggested changes for ILIAS 10/trunk. |
…ering and range. (ILIAS-eLearning#8106) * Fix https://mantis.ilias.de/view.php?id=41328 * Fix https://mantis.ilias.de/view.php?id=42095 * Remove Table\Data::withNumberOfRows() * Replace IRSS usage of removed method
https://mantis.ilias.de/view.php?id=41328
Optionally initialize a DataTable with Range and/or Order.
This includes the fix from #8083
Also, I removed withNumberOfRows/getNumberOfRows from the interface in favour of (allready existing) get-/withRange.